Skip to content

Hide verificationToken - #1851

Merged
bajtos merged 1 commit into
strongloop:masterfrom
gausie:patch-4
Jan 25, 2016
Merged

Hide verificationToken#1851
bajtos merged 1 commit into
strongloop:masterfrom
gausie:patch-4

Conversation

@gausie

@gausie gausie commented Dec 3, 2015

Copy link
Copy Markdown
Contributor

We should never be showing this publically.

@gausie

gausie commented Dec 8, 2015

Copy link
Copy Markdown
Contributor Author

I don't really understand how the automated tests work - none of them have actually failed.

@bajtos

bajtos commented Dec 14, 2015

Copy link
Copy Markdown
Member

Hi @gausie, could you please rebase your patch on top of the current master and add a unit-test covering your change? See 5b50a99 for inspiration.

As for the automated tests, our Jenkins CI server is not very stable these days. Make sure all tests pass on your machine and on Travis CI.

@bajtos bajtos self-assigned this Dec 14, 2015
@bajtos

bajtos commented Dec 14, 2015

Copy link
Copy Markdown
Member

@ritch @raymondfeng you are more familiar with user verification process than I am. Do you see any reason why it may be a bad idea to hide "verificationToken" from User JSON?

@gausie

gausie commented Dec 22, 2015

Copy link
Copy Markdown
Contributor Author

Rebased and unit test added.

@bajtos

bajtos commented Jan 5, 2016

Copy link
Copy Markdown
Member

@gausie could you please squash the commits into a single one?

Note that !userJson.verificationToken is true when the token has a falsy value. You should use !('verificationToken' in userJson) instead.

I am wondering if we need to go through the whole verify process in the unit-test, isn't it enough to set the property and then check it's not serialized to JSON?

var user = new User({email: 'bar@bat.com', password: 'bar', verificationToken: 'a-token' });
var data = user.toJSON();
assert(!('verificationToken' in data));

We should never be showing this publically.

Adds unit test for hiding verification token.
@gausie

gausie commented Jan 12, 2016

Copy link
Copy Markdown
Contributor Author

@bajtos, have followed both of your recommendations

@bajtos

bajtos commented Jan 25, 2016

Copy link
Copy Markdown
Member

@slnode test please

bajtos added a commit that referenced this pull request Jan 25, 2016
Hide verificationToken from JSON output
@bajtos
bajtos merged commit 0ad150c into strongloop:master Jan 25, 2016
@bajtos

bajtos commented Jan 25, 2016

Copy link
Copy Markdown
Member

Landed, thank you @gausie for your contribution!

bajtos added a commit that referenced this pull request Feb 4, 2016
We should never be showing this publically.

Adds unit test for hiding verification token.

This is a back-port of pull request #1851 from gausie/patch-4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants